[feat] Add ownership filter to model browser #7201
Conversation
luke-mino-altherr
requested review from
a team,
KarryCharon,
Yorha4D and
shinshin86
as code owners
dosubot
bot
added
the
size:L
📝 WalkthroughWalkthroughAdds ownership-based filtering to the asset browser: new i18n keys, an ownership SingleSelect shown when mutable assets exist, ownership filtering in the composable, fixture support for asset immutability, updated tests, and AssetBrowserModal wiring to pass all assets. Changes
sequenceDiagram
participant User
participant AssetFilterBar
participant AssetBrowserModal
participant useAssetBrowser
participant AssetStore
User->>AssetFilterBar: selects Ownership option
AssetFilterBar->>AssetBrowserModal: emit filterChange { ownership, sortBy, ... }
AssetBrowserModal->>useAssetBrowser: updateFilters({ ownership, ... })
useAssetBrowser->>AssetStore: read fetchedAssets / all assets
useAssetBrowser->>useAssetBrowser: apply file-format, base-model, ownership filters
useAssetBrowser-->>AssetBrowserModal: return filteredAssets
AssetBrowserModal-->>User: render updated asset list
Possibly related PRs
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 12/07/2025, 02:06:51 AM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🎭 Playwright Test Results⏰ Completed at: 12/07/2025, 02:15:16 AM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 3.2 MB (baseline 3.2 MB) • 🔴 +125 BMain entry bundles and manifests
Status: 3 added / 3 removed Graph Workspace — 978 kB (baseline 976 kB) • 🔴 +1.59 kBGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 6.54 kB (baseline 6.54 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 1 added / 1 removed Panels & Settings — 298 kB (baseline 298 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 6 added / 6 removed UI Components — 177 kB (baseline 177 kB) • ⚪ 0 BReusable component library chunks
Status: 6 added / 6 removed Data & Services — 12.5 kB (baseline 12.5 kB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 2 added / 2 removed Utilities & Hooks — 2.94 kB (baseline 2.94 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 1 added / 1 removed Vendor & Third-Party — 8.56 MB (baseline 8.56 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 3.81 MB (baseline 3.81 MB) • ⚪ 0 BBundles that do not match a named category
Status: 17 added / 17 removed |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests-ui/platform/assets/composables/useAssetBrowser.test.ts (1)
248-255: FilterState updates are consistent; align sortBy value with implementationIncluding
ownership: 'all'in the variousupdateFilterscalls keeps these tests in sync with the extendedFilterStateand with the composable’s default behavior.One small clean‑up: in the “sorts assets by name” test you still use
sortBy: 'name', whileuseAssetBrowser’s switch handles'name-asc' | 'name-desc' | 'recent' | 'popular'and falls back to'name-asc'for unknown values. To avoid relying on the fallback branch, consider updating that test to use'name-asc'explicitly.Also applies to: 285-290, 340-345, 365-370
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
src/locales/en/main.json(1 hunks)src/platform/assets/components/AssetFilterBar.vue(5 hunks)src/platform/assets/composables/useAssetBrowser.ts(3 hunks)src/platform/assets/fixtures/ui-mock-assets.ts(1 hunks)tests-ui/platform/assets/components/AssetFilterBar.test.ts(1 hunks)tests-ui/platform/assets/composables/useAssetBrowser.test.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (16)
tests-ui/**/*.test.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (tests-ui/CLAUDE.md)
tests-ui/**/*.test.{js,ts,jsx,tsx}: Write tests for new features
Follow existing test patterns in the codebase
Use existing test utilities rather than writing custom utilities
Mock external dependencies in tests
Always prefer vitest mock functions over writing verbose manual mocks
Files:
tests-ui/platform/assets/components/AssetFilterBar.test.tstests-ui/platform/assets/composables/useAssetBrowser.test.ts
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript exclusively; no new JavaScript code
Files:
tests-ui/platform/assets/components/AssetFilterBar.test.tssrc/platform/assets/fixtures/ui-mock-assets.tstests-ui/platform/assets/composables/useAssetBrowser.test.tssrc/platform/assets/composables/useAssetBrowser.ts
**/*.{ts,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,vue}: Use camelCase for variable and function names
Indent with 2 spaces (see.prettierrc)
Use single quotes for strings (see.prettierrc)
No trailing semicolons (see.prettierrc)
Maximum line width of 80 characters (see.prettierrc)
Sort and group imports by plugin (runpnpm formatbefore committing)
Never useanytype; use proper TypeScript types instead
Never useas anytype assertions; fix the underlying type issue instead
Avoid code comments unless absolutely necessary; write expressive, self-documenting code instead
When writing new code, ask if there is a simpler way to introduce the same functionality; if yes, choose the simpler approach
Use refactoring to make complex code simpler
Use es-toolkit for utility functions
Use Vite for fast development and building
Implement proper error handling
Write tests for all changes, especially bug fixes to catch future regressions
Files:
tests-ui/platform/assets/components/AssetFilterBar.test.tssrc/platform/assets/fixtures/ui-mock-assets.tstests-ui/platform/assets/composables/useAssetBrowser.test.tssrc/platform/assets/composables/useAssetBrowser.tssrc/platform/assets/components/AssetFilterBar.vue
**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.test.ts: Avoid writing change detector tests that just assert default values
Avoid writing tests dependent on non-behavioral features like utility classes or styles
Avoid writing redundant tests
Files:
tests-ui/platform/assets/components/AssetFilterBar.test.tstests-ui/platform/assets/composables/useAssetBrowser.test.ts
src/**/*.{vue,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{vue,ts}: Leverage VueUse functions for performance-enhancing styles
Implement proper error handling
Use vue-i18n in composition API for any string literals. Place new translation entries in src/locales/en/main.json
Files:
src/platform/assets/fixtures/ui-mock-assets.tssrc/platform/assets/composables/useAssetBrowser.tssrc/platform/assets/components/AssetFilterBar.vue
src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.ts: Use es-toolkit for utility functions
Use TypeScript for type safety
Files:
src/platform/assets/fixtures/ui-mock-assets.tssrc/platform/assets/composables/useAssetBrowser.ts
src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.{ts,tsx,vue}: Sanitize HTML with DOMPurify to prevent XSS attacks
Avoid using @ts-expect-error; use proper TypeScript types instead
Use es-toolkit for utility functions instead of other utility libraries
Implement proper TypeScript types throughout the codebase
Files:
src/platform/assets/fixtures/ui-mock-assets.tssrc/platform/assets/composables/useAssetBrowser.tssrc/platform/assets/components/AssetFilterBar.vue
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/platform/assets/fixtures/ui-mock-assets.tssrc/platform/assets/composables/useAssetBrowser.tssrc/platform/assets/components/AssetFilterBar.vue
src/locales/**/*.json
📄 CodeRabbit inference engine (AGENTS.md)
Place new i18n translation entries in
src/locales/en/main.jsonand usevue-i18nin Composition API for string literals
Files:
src/locales/en/main.json
**/composables/**/use*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name composables with
useXyz.tspattern
Files:
tests-ui/platform/assets/composables/useAssetBrowser.test.tssrc/platform/assets/composables/useAssetBrowser.ts
src/**/{services,composables}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/{services,composables}/**/*.{ts,tsx}: Useapi.apiURL()for backend endpoints instead of constructing URLs directly
Useapi.fileURL()for static file access instead of constructing URLs directly
Files:
src/platform/assets/composables/useAssetBrowser.ts
src/**/{composables,components}/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Clean up subscriptions in state management to prevent memory leaks
Files:
src/platform/assets/composables/useAssetBrowser.tssrc/platform/assets/components/AssetFilterBar.vue
src/**/{components,composables}/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Use vue-i18n for ALL user-facing strings by adding them to
src/locales/en/main.json
Files:
src/platform/assets/composables/useAssetBrowser.tssrc/platform/assets/components/AssetFilterBar.vue
src/**/*.vue
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.vue: Use the Vue 3 Composition API instead of the Options API when writing Vue components (exception: when overriding or extending PrimeVue components for compatibility)
Use setup() function for component logic
Utilize ref and reactive for reactive state
Implement computed properties with computed()
Use watch and watchEffect for side effects
Implement lifecycle hooks with onMounted, onUpdated, etc.
Utilize provide/inject for dependency injection
Use vue 3.5 style of default prop declaration
Use Tailwind CSS for styling
Implement proper props and emits definitions
Utilize Vue 3's Teleport component when needed
Use Suspense for async components
Follow Vue 3 style guide and naming conventions
Files:
src/platform/assets/components/AssetFilterBar.vue
**/*.vue
📄 CodeRabbit inference engine (AGENTS.md)
**/*.vue: Use Vue 3 SFCs with Composition API only (use<script setup lang="ts">, not Options API)
Avoid using<style>blocks; use Tailwind 4 for all styling
UsedefinePropswith TypeScript style default declaration; do not usewithDefaultsor runtime props declaration
PreferuseModelover separately defining a prop and emit
Usecomputedinstead of arefandwatchif possible
Avoid usingrefif a prop would accomplish the design goals; avoid usingcomputedif arefor prop directly would work
Do not import Vue macros unnecessarily
Never use thedark:Tailwind variant; use semantic values from thestyle.csstheme instead (e.g.,bg-node-component-surface)
Never use:class="[]"to merge class names; always usecn()from@/utils/tailwindUtil(e.g.,<div :class="cn('text-node-component-header-icon', hasError && 'text-danger')" />)
Leverage VueUse functions for performance-enhancing styles
Avoid new usage of PrimeVue components
Use Vue 3 Teleport component when needed
Use Vue 3 Suspense for async components
Files:
src/platform/assets/components/AssetFilterBar.vue
**/components/**/*.vue
📄 CodeRabbit inference engine (AGENTS.md)
Name Vue components in PascalCase (e.g.,
MenuHamburger.vue)
Files:
src/platform/assets/components/AssetFilterBar.vue
🧠 Learnings (16)
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Write tests for new features
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-11-24T19:47:22.909Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Check assets/ directory for test data when writing tests
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.tstests-ui/platform/assets/composables/useAssetBrowser.test.ts
📚 Learning: 2025-12-06T00:52:35.750Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T00:52:35.750Z
Learning: Applies to **/*.{ts,vue} : Write tests for all changes, especially bug fixes to catch future regressions
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Follow existing test patterns in the codebase
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-12-06T00:52:35.750Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T00:52:35.750Z
Learning: Applies to **/*.test.ts : Avoid writing change detector tests that just assert default values
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-12-06T00:52:35.750Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T00:52:35.750Z
Learning: Applies to **/*.test.ts : Avoid writing tests dependent on non-behavioral features like utility classes or styles
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-11-24T19:47:02.860Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T19:47:02.860Z
Learning: Applies to src/**/*.vue : Utilize ref and reactive for reactive state
Applied to files:
src/platform/assets/components/AssetFilterBar.vue
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Use ref/reactive for state management in Vue 3 Composition API
Applied to files:
src/platform/assets/components/AssetFilterBar.vue
📚 Learning: 2025-12-06T00:52:35.750Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T00:52:35.750Z
Learning: Applies to **/*.vue : Use `computed` instead of a `ref` and `watch` if possible
Applied to files:
src/platform/assets/components/AssetFilterBar.vue
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Implement computed() for derived state in Vue 3 Composition API
Applied to files:
src/platform/assets/components/AssetFilterBar.vue
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Use setup() function in Vue 3 Composition API
Applied to files:
src/platform/assets/components/AssetFilterBar.vue
📚 Learning: 2025-12-06T00:52:35.750Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T00:52:35.750Z
Learning: Applies to **/*.vue : Avoid using `ref` if a prop would accomplish the design goals; avoid using `computed` if a `ref` or prop directly would work
Applied to files:
src/platform/assets/components/AssetFilterBar.vue
📚 Learning: 2025-11-24T19:47:02.860Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T19:47:02.860Z
Learning: Applies to src/**/*.vue : Use setup() function for component logic
Applied to files:
src/platform/assets/components/AssetFilterBar.vue
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Replace PrimeVue Dropdown component with Select
Applied to files:
src/platform/assets/components/AssetFilterBar.vue
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Replace PrimeVue Chips component with AutoComplete with multiple enabled
Applied to files:
src/platform/assets/components/AssetFilterBar.vue
📚 Learning: 2025-12-06T00:52:35.750Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T00:52:35.750Z
Learning: Applies to **/*.vue : Use Vue 3 SFCs with Composition API only (use `<script setup lang="ts">`, not Options API)
Applied to files:
src/platform/assets/components/AssetFilterBar.vue
🧬 Code graph analysis (3)
tests-ui/platform/assets/components/AssetFilterBar.test.ts (1)
src/platform/assets/fixtures/ui-mock-assets.ts (1)
createAssetWithSpecificExtension(149-159)
tests-ui/platform/assets/composables/useAssetBrowser.test.ts (1)
src/platform/assets/composables/useAssetBrowser.ts (1)
useAssetBrowser(67-225)
src/platform/assets/composables/useAssetBrowser.ts (1)
src/platform/assets/schemas/assetSchema.ts (1)
AssetItem(73-73)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: deploy-and-comment
- GitHub Check: test
- GitHub Check: setup
- GitHub Check: lint-and-format
- GitHub Check: collect
🔇 Additional comments (5)
tests-ui/platform/assets/components/AssetFilterBar.test.ts (2)
251-295: Ownership filter visibility tests match the intended UXThese tests correctly assert that the ownership control is:
- Hidden when all assets are immutable,
- Shown when any asset is mutable (including mixed sets).
Using the
label === 'assetBrowser.ownership'filter makes the assertions robust against additionalSingleSelectinstances.
298-337: Good coverage of ownership emissions; consider extending the structure testThe new tests validate that:
- Changing the ownership selector emits
filterChangewith the expectedownershipvalue.ownershipdefaults to'all'when other filters (e.g., sort) change.That’s solid coverage. As a small improvement, you could extend the existing “ensures FilterState interface compliance” test to also assert that
typeof filterState.ownership === 'string'(and possibly that it’s defined), so it stays aligned when FilterState evolves.⛔ Skipped due to learnings
Learnt from: CR Repo: Comfy-Org/ComfyUI_frontend PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-12-06T00:52:35.750Z Learning: Applies to **/*.{ts,vue} : Write tests for all changes, especially bug fixes to catch future regressionstests-ui/platform/assets/composables/useAssetBrowser.test.ts (1)
381-463: Ownership filter behavior is well-covered in the composable testsThe new tests for:
'all'(no filtering),'my-models'(!is_immutable),'public-models'(is_immutable),nicely validate the core ownership logic in
useAssetBrowser. They directly exercise the newownershipfield inFilterStateand confirm that the pipeline returns the expected asset counts and predicates.src/platform/assets/components/AssetFilterBar.vue (1)
30-38: Ownership selector UI is wired correctlyThe new
SingleSelectfor ownership:
- Is gated by
hasMutableAssets, matching the requirement to show it only when mutable assets exist.- Uses the new
assetBrowser.ownershiplabel andownershipOptions.- Hooks into
v-model="ownership"and@update:model-value="handleFilterChange"in line with the existing filters.This is a good, minimal addition to the filter bar.
src/locales/en/main.json (1)
2139-2142: Ownership i18n keys correctly added and namedThe new
assetBrowser.ownership*keys are well‑named, concise, and match their usage inAssetFilterBar.vueand the ownership options. No issues here.
Add dropdown to filter models by ownership (All/My models/Public models). The filter only appears when user has uploaded models. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- Change FilterState.ownership to use OwnershipOption literal union type - Use strict comparison (=== false/true) for is_immutable checks - Move type definitions before interface to follow dependency order 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Use findAllComponents with label filter to specifically find the sort dropdown instead of the first SingleSelect, which could be the ownership filter when mutable assets are present. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Pass allAssets to AssetFilterBar to check for mutable assets across all categories, not just the currently filtered ones. This ensures the ownership filter remains visible when switching categories, providing better UX consistency. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
luke-mino-altherr
force-pushed
the
feat/model-modal-filter-by-ownership
branch
from
1dfba42 to
03791f7
Compare
🔧 Auto-fixes AppliedThis PR has been automatically updated to fix linting and formatting issues.
Changes made:
|
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/platform/assets/composables/useAssetBrowser.ts (1)
38-45: Consider using stricter typing for the ownership parameter.The filter logic is correct and handles
undefinedvalues properly with strict equality. However, theownershipparameter is typed asstringrather than using the narrower type fromFilterState['ownership'].Apply this diff for improved type safety:
-function filterByOwnership(ownership: string) { +function filterByOwnership(ownership: FilterState['ownership']) { return (asset: AssetItem) => { if (ownership === 'all') return true if (ownership === 'my-models') return asset.is_immutable === false if (ownership === 'public-models') return asset.is_immutable === true return true } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
src/locales/en/main.json(1 hunks)src/platform/assets/components/AssetBrowserModal.vue(1 hunks)src/platform/assets/components/AssetFilterBar.vue(4 hunks)src/platform/assets/composables/useAssetBrowser.ts(3 hunks)src/platform/assets/fixtures/ui-mock-assets.ts(1 hunks)tests-ui/platform/assets/components/AssetFilterBar.test.ts(2 hunks)tests-ui/platform/assets/composables/useAssetBrowser.test.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (16)
src/**/*.{vue,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{vue,ts}: Leverage VueUse functions for performance-enhancing styles
Implement proper error handling
Use vue-i18n in composition API for any string literals. Place new translation entries in src/locales/en/main.json
Files:
src/platform/assets/fixtures/ui-mock-assets.tssrc/platform/assets/components/AssetBrowserModal.vuesrc/platform/assets/composables/useAssetBrowser.tssrc/platform/assets/components/AssetFilterBar.vue
src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.ts: Use es-toolkit for utility functions
Use TypeScript for type safety
Files:
src/platform/assets/fixtures/ui-mock-assets.tssrc/platform/assets/composables/useAssetBrowser.ts
src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.{ts,tsx,vue}: Sanitize HTML with DOMPurify to prevent XSS attacks
Avoid using @ts-expect-error; use proper TypeScript types instead
Use es-toolkit for utility functions instead of other utility libraries
Implement proper TypeScript types throughout the codebase
Files:
src/platform/assets/fixtures/ui-mock-assets.tssrc/platform/assets/components/AssetBrowserModal.vuesrc/platform/assets/composables/useAssetBrowser.tssrc/platform/assets/components/AssetFilterBar.vue
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/platform/assets/fixtures/ui-mock-assets.tssrc/platform/assets/components/AssetBrowserModal.vuesrc/platform/assets/composables/useAssetBrowser.tssrc/platform/assets/components/AssetFilterBar.vue
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript exclusively; no new JavaScript code
Files:
src/platform/assets/fixtures/ui-mock-assets.tstests-ui/platform/assets/components/AssetFilterBar.test.tssrc/platform/assets/composables/useAssetBrowser.tstests-ui/platform/assets/composables/useAssetBrowser.test.ts
**/*.{ts,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,vue}: Use camelCase for variable and function names
Indent with 2 spaces (see.prettierrc)
Use single quotes for strings (see.prettierrc)
No trailing semicolons (see.prettierrc)
Maximum line width of 80 characters (see.prettierrc)
Sort and group imports by plugin (runpnpm formatbefore committing)
Never useanytype; use proper TypeScript types instead
Never useas anytype assertions; fix the underlying type issue instead
Avoid code comments unless absolutely necessary; write expressive, self-documenting code instead
When writing new code, ask if there is a simpler way to introduce the same functionality; if yes, choose the simpler approach
Use refactoring to make complex code simpler
Use es-toolkit for utility functions
Use Vite for fast development and building
Implement proper error handling
Write tests for all changes, especially bug fixes to catch future regressions
Files:
src/platform/assets/fixtures/ui-mock-assets.tstests-ui/platform/assets/components/AssetFilterBar.test.tssrc/platform/assets/components/AssetBrowserModal.vuesrc/platform/assets/composables/useAssetBrowser.tssrc/platform/assets/components/AssetFilterBar.vuetests-ui/platform/assets/composables/useAssetBrowser.test.ts
tests-ui/**/*.test.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (tests-ui/CLAUDE.md)
tests-ui/**/*.test.{js,ts,jsx,tsx}: Write tests for new features
Follow existing test patterns in the codebase
Use existing test utilities rather than writing custom utilities
Mock external dependencies in tests
Always prefer vitest mock functions over writing verbose manual mocks
Files:
tests-ui/platform/assets/components/AssetFilterBar.test.tstests-ui/platform/assets/composables/useAssetBrowser.test.ts
**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.test.ts: Avoid writing change detector tests that just assert default values
Avoid writing tests dependent on non-behavioral features like utility classes or styles
Avoid writing redundant tests
Files:
tests-ui/platform/assets/components/AssetFilterBar.test.tstests-ui/platform/assets/composables/useAssetBrowser.test.ts
src/**/*.vue
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.vue: Use the Vue 3 Composition API instead of the Options API when writing Vue components (exception: when overriding or extending PrimeVue components for compatibility)
Use setup() function for component logic
Utilize ref and reactive for reactive state
Implement computed properties with computed()
Use watch and watchEffect for side effects
Implement lifecycle hooks with onMounted, onUpdated, etc.
Utilize provide/inject for dependency injection
Use vue 3.5 style of default prop declaration
Use Tailwind CSS for styling
Implement proper props and emits definitions
Utilize Vue 3's Teleport component when needed
Use Suspense for async components
Follow Vue 3 style guide and naming conventions
Files:
src/platform/assets/components/AssetBrowserModal.vuesrc/platform/assets/components/AssetFilterBar.vue
src/**/{composables,components}/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Clean up subscriptions in state management to prevent memory leaks
Files:
src/platform/assets/components/AssetBrowserModal.vuesrc/platform/assets/composables/useAssetBrowser.tssrc/platform/assets/components/AssetFilterBar.vue
src/**/{components,composables}/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Use vue-i18n for ALL user-facing strings by adding them to
src/locales/en/main.json
Files:
src/platform/assets/components/AssetBrowserModal.vuesrc/platform/assets/composables/useAssetBrowser.tssrc/platform/assets/components/AssetFilterBar.vue
**/*.vue
📄 CodeRabbit inference engine (AGENTS.md)
**/*.vue: Use Vue 3 SFCs with Composition API only (use<script setup lang="ts">, not Options API)
Avoid using<style>blocks; use Tailwind 4 for all styling
UsedefinePropswith TypeScript style default declaration; do not usewithDefaultsor runtime props declaration
PreferuseModelover separately defining a prop and emit
Usecomputedinstead of arefandwatchif possible
Avoid usingrefif a prop would accomplish the design goals; avoid usingcomputedif arefor prop directly would work
Do not import Vue macros unnecessarily
Never use thedark:Tailwind variant; use semantic values from thestyle.csstheme instead (e.g.,bg-node-component-surface)
Never use:class="[]"to merge class names; always usecn()from@/utils/tailwindUtil(e.g.,<div :class="cn('text-node-component-header-icon', hasError && 'text-danger')" />)
Leverage VueUse functions for performance-enhancing styles
Avoid new usage of PrimeVue components
Use Vue 3 Teleport component when needed
Use Vue 3 Suspense for async components
Files:
src/platform/assets/components/AssetBrowserModal.vuesrc/platform/assets/components/AssetFilterBar.vue
**/components/**/*.vue
📄 CodeRabbit inference engine (AGENTS.md)
Name Vue components in PascalCase (e.g.,
MenuHamburger.vue)
Files:
src/platform/assets/components/AssetBrowserModal.vuesrc/platform/assets/components/AssetFilterBar.vue
src/**/{services,composables}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/{services,composables}/**/*.{ts,tsx}: Useapi.apiURL()for backend endpoints instead of constructing URLs directly
Useapi.fileURL()for static file access instead of constructing URLs directly
Files:
src/platform/assets/composables/useAssetBrowser.ts
**/composables/**/use*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name composables with
useXyz.tspattern
Files:
src/platform/assets/composables/useAssetBrowser.tstests-ui/platform/assets/composables/useAssetBrowser.test.ts
src/locales/**/*.json
📄 CodeRabbit inference engine (AGENTS.md)
Place new i18n translation entries in
src/locales/en/main.jsonand usevue-i18nin Composition API for string literals
Files:
src/locales/en/main.json
🧠 Learnings (15)
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Write tests for new features
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.tstests-ui/platform/assets/composables/useAssetBrowser.test.ts
📚 Learning: 2025-11-24T19:47:22.909Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Check assets/ directory for test data when writing tests
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.tstests-ui/platform/assets/composables/useAssetBrowser.test.ts
📚 Learning: 2025-11-24T19:47:22.909Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Prefer specific selectors in browser tests
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-12-06T00:52:35.750Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T00:52:35.750Z
Learning: Applies to **/*.{ts,vue} : Write tests for all changes, especially bug fixes to catch future regressions
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Follow existing test patterns in the codebase
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-11-24T19:47:02.860Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T19:47:02.860Z
Learning: Applies to src/**/*.vue : Utilize ref and reactive for reactive state
Applied to files:
src/platform/assets/components/AssetFilterBar.vue
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Use ref/reactive for state management in Vue 3 Composition API
Applied to files:
src/platform/assets/components/AssetFilterBar.vue
📚 Learning: 2025-12-06T00:52:35.750Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T00:52:35.750Z
Learning: Applies to **/*.vue : Use `computed` instead of a `ref` and `watch` if possible
Applied to files:
src/platform/assets/components/AssetFilterBar.vue
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Replace PrimeVue Dropdown component with Select
Applied to files:
src/platform/assets/components/AssetFilterBar.vue
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Use setup() function in Vue 3 Composition API
Applied to files:
src/platform/assets/components/AssetFilterBar.vue
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Replace PrimeVue Chips component with AutoComplete with multiple enabled
Applied to files:
src/platform/assets/components/AssetFilterBar.vue
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Implement computed() for derived state in Vue 3 Composition API
Applied to files:
src/platform/assets/components/AssetFilterBar.vue
📚 Learning: 2025-12-06T00:52:35.750Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T00:52:35.750Z
Learning: Applies to **/*.vue : Use Vue 3 SFCs with Composition API only (use `<script setup lang="ts">`, not Options API)
Applied to files:
src/platform/assets/components/AssetFilterBar.vue
📚 Learning: 2025-11-24T19:47:02.860Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T19:47:02.860Z
Learning: Applies to src/**/*.vue : Use setup() function for component logic
Applied to files:
src/platform/assets/components/AssetFilterBar.vue
📚 Learning: 2025-12-06T00:52:35.750Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T00:52:35.750Z
Learning: Applies to **/*.vue : Avoid using `ref` if a prop would accomplish the design goals; avoid using `computed` if a `ref` or prop directly would work
Applied to files:
src/platform/assets/components/AssetFilterBar.vue
🧬 Code graph analysis (3)
tests-ui/platform/assets/components/AssetFilterBar.test.ts (1)
src/platform/assets/fixtures/ui-mock-assets.ts (1)
createAssetWithSpecificExtension(149-159)
src/platform/assets/composables/useAssetBrowser.ts (1)
src/platform/assets/schemas/assetSchema.ts (1)
AssetItem(73-73)
tests-ui/platform/assets/composables/useAssetBrowser.test.ts (1)
src/platform/assets/composables/useAssetBrowser.ts (1)
useAssetBrowser(67-225)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: test
- GitHub Check: setup
- GitHub Check: lint-and-format
- GitHub Check: collect
🔇 Additional comments (18)
src/platform/assets/fixtures/ui-mock-assets.ts (1)
149-159: LGTM!The optional
isImmutableparameter cleanly extends the fixture helper to support ownership-related tests while maintaining backward compatibility with existing call sites.src/platform/assets/components/AssetBrowserModal.vue (1)
49-53: LGTM!Correctly passes the full asset set to
AssetFilterBarviaall-assets, enabling the ownership filter visibility check to operate on all fetched assets rather than just the category-filtered subset.src/locales/en/main.json (1)
2139-2142: LGTM!New i18n keys for the ownership filter are appropriately placed in the
assetBrowsersection and follow the existing camelCase naming convention.src/platform/assets/composables/useAssetBrowser.ts (2)
74-79: LGTM!The filter state correctly initializes
ownershipto'all', ensuring all models are shown by default as specified in the PR objectives.
185-190: LGTM!The ownership filter is correctly integrated into the filtering pipeline, applied after file format and base model filters, maintaining consistency with the existing filter chain pattern.
tests-ui/platform/assets/composables/useAssetBrowser.test.ts (4)
381-405: Test coverage for 'all' ownership is correct.The test properly verifies that when ownership is set to
'all', all assets are returned regardless of theiris_immutablestate.
407-434: Good assertion pattern for 'my-models' filter.The test correctly verifies both the count and that all returned assets have
is_immutable === false(mutable assets).
436-463: Good assertion pattern for 'public-models' filter.The test correctly verifies both the count and that all returned assets have
is_immutable === true(immutable/public assets).
249-254: LGTM!Existing tests correctly updated to include the new
ownershipfield in filter updates.tests-ui/platform/assets/components/AssetFilterBar.test.ts (3)
253-266: LGTM!The test correctly verifies that the ownership filter is hidden when all assets are immutable.
268-297: LGTM!These tests provide good coverage for ownership filter visibility with both mutable-only and mixed asset scenarios.
300-340: Comprehensive ownership filter behavior coverage.These tests properly verify that ownership changes are emitted and that the default value is 'all'. The non-null assertion concern mentioned in the earlier comment also applies here.
src/platform/assets/components/AssetFilterBar.vue (6)
30-38: LGTM!The ownership filter UI follows the established pattern for SingleSelect components and correctly uses conditional rendering based on
hasMutableAssets.
59-59: LGTM!The
computedimport is correctly added and necessary for thehasMutableAssetscomputed property.
78-86: LGTM!The ownership options follow the same pattern as
SORT_OPTIONS, providing excellent type safety with the derivedOwnershipOptionliteral union type.
88-93: LGTM!The
FilterStateinterface correctly typesownershipasOwnershipOptionrather than a plain string, providing type safety for consumers. This addresses the past review feedback appropriately.
95-110: LGTM!The implementation correctly:
- Adds
allAssetsprop for visibility logic (separate from filteredassets)- Initializes
ownershipto 'all' as the default- Uses strict comparison (
=== false) inhasMutableAssetsto treat undefined/missingis_immutableas immutableThis addresses the past review feedback about explicit mutability checks.
120-121: LGTM!The
handleFilterChangefunction correctly includesownershipin the emitted filter state, maintaining consistency with the other filter fields.
- Add explicit toBeTruthy() checks before using non-null assertions to provide clear error messages when components aren't found - Move ownership filtering tests from 'Sorting' to dedicated 'Ownership filtering' describe block for better organization 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
The ownership filter visibility depends on the allAssets prop to determine if mutable assets exist. Updated test cases to pass both assets and allAssets props to properly test the ownership filter functionality.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
tests-ui/platform/assets/components/AssetFilterBar.test.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
tests-ui/**/*.test.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (tests-ui/CLAUDE.md)
tests-ui/**/*.test.{js,ts,jsx,tsx}: Write tests for new features
Follow existing test patterns in the codebase
Use existing test utilities rather than writing custom utilities
Mock external dependencies in tests
Always prefer vitest mock functions over writing verbose manual mocks
Files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript exclusively; no new JavaScript code
Files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
**/*.{ts,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,vue}: Use camelCase for variable and function names
Indent with 2 spaces (see.prettierrc)
Use single quotes for strings (see.prettierrc)
No trailing semicolons (see.prettierrc)
Maximum line width of 80 characters (see.prettierrc)
Sort and group imports by plugin (runpnpm formatbefore committing)
Never useanytype; use proper TypeScript types instead
Never useas anytype assertions; fix the underlying type issue instead
Avoid code comments unless absolutely necessary; write expressive, self-documenting code instead
When writing new code, ask if there is a simpler way to introduce the same functionality; if yes, choose the simpler approach
Use refactoring to make complex code simpler
Use es-toolkit for utility functions
Use Vite for fast development and building
Implement proper error handling
Write tests for all changes, especially bug fixes to catch future regressions
Files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.test.ts: Avoid writing change detector tests that just assert default values
Avoid writing tests dependent on non-behavioral features like utility classes or styles
Avoid writing redundant tests
Files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
🧠 Learnings (10)
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Write tests for new features
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-11-24T19:47:22.909Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Check assets/ directory for test data when writing tests
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-12-06T00:52:35.750Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T00:52:35.750Z
Learning: Applies to **/*.{ts,vue} : Write tests for all changes, especially bug fixes to catch future regressions
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-11-24T19:47:22.909Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Prefer specific selectors in browser tests
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Follow existing test patterns in the codebase
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-12-06T00:52:35.750Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T00:52:35.750Z
Learning: Applies to **/*.test.ts : Avoid writing change detector tests that just assert default values
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-12-06T00:52:35.750Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T00:52:35.750Z
Learning: Applies to **/*.test.ts : Avoid writing tests dependent on non-behavioral features like utility classes or styles
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-11-24T19:47:34.324Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:34.324Z
Learning: Applies to src/**/*.{ts,tsx,vue} : Avoid using ts-expect-error; use proper TypeScript types instead
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-12-06T00:52:35.750Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T00:52:35.750Z
Learning: Applies to **/*.test.ts : Avoid writing redundant tests
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
📚 Learning: 2025-12-06T00:52:35.750Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T00:52:35.750Z
Learning: Applies to **/*.{ts,vue} : Never use `as any` type assertions; fix the underlying type issue instead
Applied to files:
tests-ui/platform/assets/components/AssetFilterBar.test.ts
🧬 Code graph analysis (1)
tests-ui/platform/assets/components/AssetFilterBar.test.ts (1)
src/platform/assets/fixtures/ui-mock-assets.ts (1)
createAssetWithSpecificExtension(149-159)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: setup
- GitHub Check: lint-and-format
- GitHub Check: test
- GitHub Check: collect
🔇 Additional comments (1)
tests-ui/platform/assets/components/AssetFilterBar.test.ts (1)
108-112: Sort select lookup pattern looks solidFinding the
SingleSelectby itslabeland asserting its existence before emitting keeps the test robust against component order changes and avoids cryptic failures. The remaining non‑null assertion is acceptable here given the explicitexpect.
| it('hides ownership filter when no mutable assets', () => { | ||
| const assets = [ | ||
| createAssetWithSpecificExtension('safetensors', true) // immutable | ||
| ] | ||
| const wrapper = mountAssetFilterBar({ assets }) | ||
|
|
||
| const ownershipSelects = wrapper | ||
| .findAllComponents({ name: 'SingleSelect' }) | ||
| .filter( | ||
| (component) => component.props('label') === 'assetBrowser.ownership' | ||
| ) | ||
|
|
||
| expect(ownershipSelects).toHaveLength(0) | ||
| }) | ||
|
|
||
| it('shows ownership filter when mutable assets exist', () => { | ||
| const assets = [ | ||
| createAssetWithSpecificExtension('safetensors', false) // mutable | ||
| ] | ||
| const wrapper = mountAssetFilterBar({ assets, allAssets: assets }) | ||
|
|
||
| const ownershipSelects = wrapper | ||
| .findAllComponents({ name: 'SingleSelect' }) | ||
| .filter( | ||
| (component) => component.props('label') === 'assetBrowser.ownership' | ||
| ) | ||
|
|
||
| expect(ownershipSelects).toHaveLength(1) | ||
| }) | ||
|
|
||
| it('shows ownership filter when mixed assets exist', () => { | ||
| const assets = [ | ||
| createAssetWithSpecificExtension('safetensors', true), // immutable | ||
| createAssetWithSpecificExtension('ckpt', false) // mutable | ||
| ] | ||
| const wrapper = mountAssetFilterBar({ assets, allAssets: assets }) | ||
|
|
||
| const ownershipSelects = wrapper | ||
| .findAllComponents({ name: 'SingleSelect' }) | ||
| .filter( | ||
| (component) => component.props('label') === 'assetBrowser.ownership' | ||
| ) | ||
|
|
||
| expect(ownershipSelects).toHaveLength(1) | ||
| }) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Align allAssets usage in ownership visibility tests
The visibility tests correctly cover “no mutable”, “all mutable”, and “mixed” cases. For the “no mutable assets” case you only pass assets, while the other two pass both assets and allAssets. To better mirror real usage (where AssetBrowserModal always supplies allAssets) and make the intent explicit, consider also wiring allAssets there:
- const wrapper = mountAssetFilterBar({ assets })
+ const wrapper = mountAssetFilterBar({ assets, allAssets: assets })This keeps the test focused on “no mutable in the full asset set” rather than on the implicit defaulting behavior of allAssets.
🤖 Prompt for AI Agents
In tests-ui/platform/assets/components/AssetFilterBar.test.ts around lines 254
to 298, the first test ("hides ownership filter when no mutable assets") only
passes assets but not allAssets, making it rely on implicit defaulting; update
that test to pass allAssets: assets (i.e., mountAssetFilterBar({ assets,
allAssets: assets })) so the test explicitly mirrors real usage where
AssetBrowserModal supplies allAssets and verifies there are no mutable assets in
the full set.
| describe('Ownership Filter', () => { | ||
| it('emits ownership filter changes', async () => { | ||
| const assets = [ | ||
| createAssetWithSpecificExtension('safetensors', false) // mutable | ||
| ] | ||
| const wrapper = mountAssetFilterBar({ assets, allAssets: assets }) | ||
|
|
||
| const ownershipSelect = wrapper | ||
| .findAllComponents({ name: 'SingleSelect' }) | ||
| .find( | ||
| (component) => component.props('label') === 'assetBrowser.ownership' | ||
| ) | ||
|
|
||
| expect(ownershipSelect).toBeTruthy() | ||
| await ownershipSelect!.vm.$emit('update:modelValue', 'my-models') | ||
| await nextTick() | ||
|
|
||
| const emitted = wrapper.emitted('filterChange') | ||
| expect(emitted).toHaveLength(1) | ||
|
|
||
| const filterState = emitted![0][0] as FilterState | ||
| expect(filterState.ownership).toBe('my-models') | ||
| }) | ||
|
|
||
| it('ownership filter defaults to "all"', async () => { | ||
| const assets = [ | ||
| createAssetWithSpecificExtension('safetensors', false) // mutable | ||
| ] | ||
| const wrapper = mountAssetFilterBar({ assets }) | ||
|
|
||
| const sortSelect = wrapper | ||
| .findAllComponents({ name: 'SingleSelect' }) | ||
| .find((component) => component.props('label') === 'assetBrowser.sortBy') | ||
|
|
||
| expect(sortSelect).toBeTruthy() | ||
| await sortSelect!.vm.$emit('update:modelValue', 'recent') | ||
| await nextTick() | ||
|
|
||
| const emitted = wrapper.emitted('filterChange') | ||
| const filterState = emitted![0][0] as FilterState | ||
| expect(filterState.ownership).toBe('all') | ||
| }) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Ownership behavior tests are good; consider small consistency tweaks
The new tests correctly assert that ownership changes propagate into filterChange and that ownership defaults to 'all' when only sort changes. Two optional polish points:
- In the default test, also assert the emitted length for symmetry and clearer failures:
- const emitted = wrapper.emitted('filterChange')
- const filterState = emitted![0][0] as FilterState
+ const emitted = wrapper.emitted('filterChange')
+ expect(emitted).toHaveLength(1)
+ const filterState = emitted![0][0] as FilterState- Optionally pass
allAssets: assetsin the default test as well to match production wiring, though behavior here doesn’t currently depend on it.
These are minor and non‑blocking; behavior under test is covered well. Based on learnings, this keeps tests behavior‑focused rather than relying on incidental defaults.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| describe('Ownership Filter', () => { | |
| it('emits ownership filter changes', async () => { | |
| const assets = [ | |
| createAssetWithSpecificExtension('safetensors', false) // mutable | |
| ] | |
| const wrapper = mountAssetFilterBar({ assets, allAssets: assets }) | |
| const ownershipSelect = wrapper | |
| .findAllComponents({ name: 'SingleSelect' }) | |
| .find( | |
| (component) => component.props('label') === 'assetBrowser.ownership' | |
| ) | |
| expect(ownershipSelect).toBeTruthy() | |
| await ownershipSelect!.vm.$emit('update:modelValue', 'my-models') | |
| await nextTick() | |
| const emitted = wrapper.emitted('filterChange') | |
| expect(emitted).toHaveLength(1) | |
| const filterState = emitted![0][0] as FilterState | |
| expect(filterState.ownership).toBe('my-models') | |
| }) | |
| it('ownership filter defaults to "all"', async () => { | |
| const assets = [ | |
| createAssetWithSpecificExtension('safetensors', false) // mutable | |
| ] | |
| const wrapper = mountAssetFilterBar({ assets }) | |
| const sortSelect = wrapper | |
| .findAllComponents({ name: 'SingleSelect' }) | |
| .find((component) => component.props('label') === 'assetBrowser.sortBy') | |
| expect(sortSelect).toBeTruthy() | |
| await sortSelect!.vm.$emit('update:modelValue', 'recent') | |
| await nextTick() | |
| const emitted = wrapper.emitted('filterChange') | |
| const filterState = emitted![0][0] as FilterState | |
| expect(filterState.ownership).toBe('all') | |
| }) | |
| describe('Ownership Filter', () => { | |
| it('emits ownership filter changes', async () => { | |
| const assets = [ | |
| createAssetWithSpecificExtension('safetensors', false) // mutable | |
| ] | |
| const wrapper = mountAssetFilterBar({ assets, allAssets: assets }) | |
| const ownershipSelect = wrapper | |
| .findAllComponents({ name: 'SingleSelect' }) | |
| .find( | |
| (component) => component.props('label') === 'assetBrowser.ownership' | |
| ) | |
| expect(ownershipSelect).toBeTruthy() | |
| await ownershipSelect!.vm.$emit('update:modelValue', 'my-models') | |
| await nextTick() | |
| const emitted = wrapper.emitted('filterChange') | |
| expect(emitted).toHaveLength(1) | |
| const filterState = emitted![0][0] as FilterState | |
| expect(filterState.ownership).toBe('my-models') | |
| }) | |
| it('ownership filter defaults to "all"', async () => { | |
| const assets = [ | |
| createAssetWithSpecificExtension('safetensors', false) // mutable | |
| ] | |
| const wrapper = mountAssetFilterBar({ assets }) | |
| const sortSelect = wrapper | |
| .findAllComponents({ name: 'SingleSelect' }) | |
| .find((component) => component.props('label') === 'assetBrowser.sortBy') | |
| expect(sortSelect).toBeTruthy() | |
| await sortSelect!.vm.$emit('update:modelValue', 'recent') | |
| await nextTick() | |
| const emitted = wrapper.emitted('filterChange') | |
| expect(emitted).toHaveLength(1) | |
| const filterState = emitted![0][0] as FilterState | |
| expect(filterState.ownership).toBe('all') | |
| }) |
🤖 Prompt for AI Agents
In tests-ui/platform/assets/components/AssetFilterBar.test.ts around lines 301
to 342, add a check in the "ownership filter defaults to 'all'" test to assert
the number of emitted 'filterChange' events (e.g.,
expect(emitted).toHaveLength(1)) for symmetry with the other test and clearer
failures, and optionally pass allAssets: assets into mountAssetFilterBar(...) to
mirror production wiring; update the test to capture emitted, assert its length,
then assert filterState.ownership === 'all'.
Summary
Adds a dropdown filter to the model browser that allows users to filter assets by ownership (All, My models, Public models), based on the
is_immutableproperty.Changes
filterByOwnershipfunction in useAssetBrowser.ts to filter byis_immutablepropertyReview Focus
!is_immutable)🤖 Generated with Claude Code
┆Issue is synchronized with this Notion page by Unito